fix(global-cli): bound local vite-plus resolution to the workspace root - #2574
fix(global-cli): bound local vite-plus resolution to the workspace root#2574Akokk0 wants to merge 11 commits into
Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The red CI is a real regression from this PR. All 75 snapshot failures have the same cause: the snapshot harness links the only real I'll push a narrower fix: the boundary only applies when the project itself declares a |
fengmk2
added
test: e2e
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
The remaining red is this PR: the same 3 snapshot cases on every platform — +warn: No project-local vite-plus installation was found. Run vp install in to install dependencies. Cause: these fixtures declare I don't know how to resolve this. |
|
@Akokk0 Let me take over. |
Local CLI resolution (oxc_resolver in the JS executor) and the `vp --version` "Local vite-plus" probe both walk every ancestor directory's node_modules, Node-style. When the project's own install is missing or broken (e.g. after a corrupted install), resolution escapes the project and silently picks up an unrelated ancestor project's copy: delegation then runs another project's vite-plus, and `vp --version` reports that copy's version and bundled tool versions as "Local". Bound the walk at the project's workspace root via `vt_workspace::find_workspace_root`: - within the workspace, nearest wins - a workspace member still resolves the workspace root's install; - beyond it, resolution fails, so delegation falls back to the global installation and the existing missing-local-cli warning (voidzero-dev#2361) explains the state instead of masking it; - when there is no workspace or package root at all, the walk stays unbounded (unchanged behavior for markerless directories). `find_local_vite_plus` in version.rs now derives from the same bounded walk, so what --version displays is what delegation would execute. Tested: unit tests cover the escape (red without the gate), the workspace-member case, and the markerless case; verified end-to-end with a nested-project fixture where 0.3.0 reports the outer project's copy and the patched build reports "Not found". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test's premise is that no ancestor of the tempdir carries a package.json. That holds for /tmp and /var/folders, but Windows' %TEMP% lives under the user profile, where a stray package.json would create a workspace boundary and fail the test for environmental reasons. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
the previous commit bounded local resolution at the workspace root for every project. that breaks a layout the repo itself relies on: the snapshot harness stages workspaces with no node_modules of their own and resolves the run-root install through Node's unbounded upward walk, so all three CLI snapshot jobs went red with the same signature - the global CLI stopped seeing the project-local install (75 cases, every diff a "does not use vite-plus" warning) walking past the package root is ordinary Node resolution semantics and hoisted installs depend on it, so the default stays unbounded. the boundary now applies only when the project declares a vite-plus dependency - directly or at its workspace root, the same test warn_missing_local_cli_if_project uses - because that is exactly the case where "run vp install" is the right answer rather than silently borrowing an unrelated ancestor's copy - new test pins the harness-shaped layout: an undeclared staged workspace keeps resolving the run-root install (mutation-verified: removing the declaration filter reds it) - the workspace-member test's root now declares the dependency so the bounded walk is actually engaged rather than passing via the unbounded default - snapshot fixtures do not declare vite-plus, so they take the unbounded path; the declared-but-missing fixture resolves its own install at the first hop either way Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b38e8bf to
edd2f76
Compare
2 participants
A nested project can run an ancestor's
vite-pluswhen its own installation is missing.vp --versioncan also display that unrelated installation.Use
oxc_resolverwithRestriction::Fnto enforce the package or workspace boundary. Workspace members can use the root installation. Excluded projects that declarevite-plusstop at their own package root. Undeclared projects retain ancestor lookup.Command delegation and
vp --versionshare the same resolver result. The restriction checks the lookup path before Oxc resolves symlinks, so local links to external package stores remain valid.Add unit tests and direct CLI output snapshots for boundaries, exports, malformed manifests, and symlinks. Link the local CLI inside migration fixtures that need it. Use an existing shell in installer tests to avoid
ETXTBSYon Linux and support BusyBox.An existing limitation remains: malformed ancestor manifests can prevent resolution in a workspace without a root
package.json.